fix: Multi turn evaluation causes JSON serialization error#62
Conversation
c6dca9e to
de0f896
Compare
de0f896 to
2bbef0d
Compare
|
|
||
| resp = proxy_conn().post("function/invoke", json=request, headers=headers, stream=stream) | ||
| request = bt_dumps(request) | ||
| resp = proxy_conn().post("function/invoke", data=request, headers=headers, stream=stream) |
There was a problem hiding this comment.
so it's important to note that switching from json=request to data=request will mean we no longer set Content-Type: application/json when sending the request. I'd double check if this is okay. If it is, feel free to merge!
Abhijeet Prasad (AbhiPrasad)
left a comment
There was a problem hiding this comment.
Oh yeah, let's also add a regression test for this - I'm actually going to block until we add that.
Co-authored-by: Abhijeet Prasad <[email protected]>
Currently tests OopenAI, Anthropic, Google
c104e90 to
9c510c5
Compare
Test added, please review it |
Abhijeet Prasad (AbhiPrasad)
left a comment
There was a problem hiding this comment.
The test you added doesn't actually test invoke. Let's mock proxy_conn and test calling invoke directly.
| ) | ||
| ) | ||
| except ImportError: | ||
| print("Anthropic not imported") |
There was a problem hiding this comment.
I would prefer if we wrote a test for each provider, and use https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest-importorskip.
openai.Client().responses.with_raw_response now has tracing Test added, very similar to the tests for the non raw responses Deals with the create and parse methods
|
woooops |
|
wrong branch |
…of github.com:braintrustdata/braintrust-sdk-python into cedric/json-serialization-error-multi-turn-evaluation
Fix #38
Deserialize JSON using Braintrust custom deserialization function (bt_dumps from bt_json), allowing to deserialize the ChatCompletionMessage objects.